Skip to content

Conversation

@millotp
Copy link
Collaborator

@millotp millotp commented Sep 22, 2024

🧭 What and Why

Because network=host doesn't work properly on mac, we need to use host.docker.internal locally and localhost on the CI.
Instead of generating the tests with the correct host, we can guess it at runtime based on the CI var env, this means we don't need to generate the tests anymore to run locally, the same code can run anywhere now !

Kotlin and ruby are a bit of a pain because the error message also contains the address, but we can replace it too.
I add to use a weird %localhost% otherwise mustache interprets it as a variable, or kotlin interprets it as a regex reference.

@millotp millotp self-assigned this Sep 22, 2024
@millotp millotp requested a review from a team as a code owner September 22, 2024 19:22
@millotp millotp requested review from Fluf22 and shortcuts September 22, 2024 19:22
@algolia-bot
Copy link
Collaborator

algolia-bot commented Sep 22, 2024

✔️ Code generated!

Name Link
🪓 Triggered by be2e708d51a6a699d558cfd34cbf7c81784d128e
🍃 Generated commit fcbc5f79656ba7e20418c0d8077520b24f820574
🌲 Generated branch generated/chore/localhost
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
javascript 1877
php 1581
go 1472
csharp 1306
java 1150
python 1071
ruby 921
swift 777

Fluf22
Fluf22 previously approved these changes Sep 22, 2024
Copy link
Collaborator

@Fluf22 Fluf22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's so kind of you to have implemented this!
It was so much painful to have to edit it manually

@millotp
Copy link
Collaborator Author

millotp commented Sep 22, 2024

It was so much painful to have to edit it manually

it would have been better to have a proxy or something but I didn't manage to make it work, this will do for now

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never had an issue with the previous implem but this one looks good too, nice :)

@millotp millotp enabled auto-merge (squash) September 22, 2024 20:46
@millotp millotp disabled auto-merge September 22, 2024 21:02
@millotp millotp enabled auto-merge (squash) September 22, 2024 21:05
@millotp millotp merged commit 428d85d into main Sep 22, 2024
55 checks passed
@millotp millotp deleted the chore/localhost branch September 22, 2024 21:19
algolia-bot added a commit that referenced this pull request Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants